From 7ab18e3aeb6f6416db2ca36e2fa2a007ea07dbfe Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 20 Nov 2015 10:46:43 -0800 Subject: [PATCH] Add a macro for `return Err(human(format!(...)))` This pattern showed up quite a few locations throughout the codebase but it ended up meaning that there were some massive levels of indentation when you actually get to the formatting string. This commit adds a new `bail!` macro which shortens this idiom to help get the indentation under control throughout the codebase. --- src/cargo/core/package_id_spec.rs | 10 +++--- src/cargo/core/resolver/mod.rs | 19 ++++------- src/cargo/core/shell.rs | 9 +++-- src/cargo/core/summary.rs | 31 +++++++---------- src/cargo/lib.rs | 6 ++++ src/cargo/ops/cargo_clean.rs | 2 +- src/cargo/ops/cargo_compile.rs | 36 ++++++++------------ src/cargo/ops/cargo_doc.rs | 12 +++---- src/cargo/ops/cargo_generate_lockfile.rs | 9 +++-- src/cargo/ops/cargo_install.rs | 14 ++++---- src/cargo/ops/cargo_new.rs | 10 +++--- src/cargo/ops/cargo_package.rs | 12 +++---- src/cargo/ops/cargo_pkgid.rs | 4 +-- src/cargo/ops/cargo_run.rs | 15 ++++----- src/cargo/ops/cargo_rustc/context.rs | 4 +-- src/cargo/ops/cargo_rustc/custom_build.rs | 15 ++++----- src/cargo/ops/cargo_rustc/links.rs | 41 +++++++++-------------- src/cargo/ops/registry.rs | 22 +++++------- src/cargo/sources/registry.rs | 11 +++--- src/cargo/util/config.rs | 11 +++--- src/cargo/util/important_paths.rs | 8 ++--- src/cargo/util/toml.rs | 14 ++++---- tests/support/mod.rs | 4 --- tests/test_cargo_compile.rs | 4 +-- tests/test_cargo_doc.rs | 2 +- tests/test_cargo_new.rs | 2 +- tests/test_cargo_registry.rs | 4 +-- tests/test_cargo_rustdoc.rs | 2 +- 28 files changed, 141 insertions(+), 192 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index 15b191e96..2520ba6a0 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -36,8 +36,7 @@ impl PackageIdSpec { }; for ch in name.chars() { if !ch.is_alphanumeric() && ch != '_' && ch != '-' { - return Err(human(format!("invalid character in pkgid `{}`: `{}`", - spec, ch))) + bail!("invalid character in pkgid `{}`: `{}`", spec, ch) } } Ok(PackageIdSpec { @@ -66,8 +65,7 @@ impl PackageIdSpec { fn from_url(mut url: Url) -> CargoResult { if url.query.is_some() { - return Err(human(format!("cannot have a query string in a pkgid: {}", - url))); + bail!("cannot have a query string in a pkgid: {}", url) } let frag = url.fragment.take(); let (name, version) = { @@ -133,8 +131,8 @@ impl PackageIdSpec { let mut ids = i.into_iter().filter(|p| self.matches(*p)); let ret = match ids.next() { Some(id) => id, - None => return Err(human(format!("package id specification `{}` \ - matched no packages", self))), + None => bail!("package id specification `{}` \ + matched no packages", self), }; return match ids.next() { Some(other) => { diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 465bc1a73..547d7e23f 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -175,8 +175,7 @@ fn activate(cx: &mut Context, // packages we're visiting and bail if we hit a dupe. let id = parent.package_id().clone(); if !cx.visited.insert(id.clone()) { - return Err(human(format!("cyclic package dependency: package `{}` \ - depends on itself", id))) + bail!("cyclic package dependency: package `{}` depends on itself", id) } // If we're already activated, then that was easy! @@ -557,9 +556,8 @@ fn build_features(s: &Summary, method: &Method) None => { let feat = feat_or_package; if !visited.insert(feat.to_string()) { - return Err(human(format!("Cyclic feature dependency: \ - feature `{}` depends on itself", - feat))) + bail!("Cyclic feature dependency: feature `{}` depends \ + on itself", feat) } used.insert(feat.to_string()); match s.features().get(feat) { @@ -677,10 +675,8 @@ impl Context { for feature in dep.features().iter() { base.push(feature.clone()); if feature.contains("/") { - return Err(human(format!("features in dependencies \ - cannot enable features in \ - other dependencies: `{}`", - feature))); + bail!("features in dependencies cannot enable features in \ + other dependencies: `{}`", feature) } } ret.push((dep.clone(), base)); @@ -695,9 +691,8 @@ impl Context { .collect::>(); if unknown.len() > 0 { let features = unknown.connect(", "); - return Err(human(format!("Package `{}` does not have these \ - features: `{}`", parent.package_id(), - features))) + bail!("Package `{}` does not have these features: `{}`", + parent.package_id(), features) } } diff --git a/src/cargo/core/shell.rs b/src/cargo/core/shell.rs index 6e0a19564..5c03d34c1 100644 --- a/src/cargo/core/shell.rs +++ b/src/cargo/core/shell.rs @@ -10,7 +10,7 @@ use self::AdequateTerminal::{NoColor, Colored}; use self::Verbosity::{Verbose, Normal, Quiet}; use self::ColorConfig::{Auto, Always, Never}; -use util::errors::{human, CargoResult}; +use util::errors::CargoResult; #[derive(Clone, Copy, PartialEq)] pub enum Verbosity { @@ -105,7 +105,7 @@ impl MultiShell { pub fn set_verbosity(&mut self, verbose: bool, quiet: bool) -> CargoResult<()> { self.verbosity = match (verbose, quiet) { - (true, true) => return Err(human("cannot set both --verbose and --quiet")), + (true, true) => bail!("cannot set both --verbose and --quiet"), (true, false) => Verbose, (false, true) => Quiet, (false, false) => Normal @@ -130,9 +130,8 @@ impl MultiShell { None => Auto, - Some(arg) => return Err(human(format!("argument for --color must be auto, always, or \ - never, but found `{}`", - arg))), + Some(arg) => bail!("argument for --color must be auto, always, or \ + never, but found `{}`", arg), }); Ok(()) } diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index 81445d2a3..01697171d 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -4,7 +4,7 @@ use std::mem; use semver::Version; use core::{Dependency, PackageId, SourceId}; -use util::{CargoResult, human}; +use util::CargoResult; /// Subset of a `Manifest`. Contains only the most important informations about /// a package. @@ -23,13 +23,12 @@ impl Summary { features: HashMap>) -> CargoResult { for dep in dependencies.iter() { if features.get(dep.name()).is_some() { - return Err(human(format!("Features and dependencies cannot have \ - the same name: `{}`", dep.name()))) + bail!("Features and dependencies cannot have the \ + same name: `{}`", dep.name()) } if dep.is_optional() && !dep.is_transitive() { - return Err(human(format!("Dev-dependencies are not allowed \ - to be optional: `{}`", - dep.name()))) + bail!("Dev-dependencies are not allowed to be optional: `{}`", + dep.name()) } } for (feature, list) in features.iter() { @@ -41,22 +40,18 @@ impl Summary { match dependencies.iter().find(|d| d.name() == dep) { Some(d) => { if d.is_optional() || is_reexport { continue } - return Err(human(format!("Feature `{}` depends on `{}` \ - which is not an optional \ - dependency.\nConsider adding \ - `optional = true` to the \ - dependency", feature, dep))) + bail!("Feature `{}` depends on `{}` which is not an \ + optional dependency.\nConsider adding \ + `optional = true` to the dependency", + feature, dep) } None if is_reexport => { - return Err(human(format!("Feature `{}` requires `{}` \ - which is not an optional \ - dependency", feature, dep))) + bail!("Feature `{}` requires `{}` which is not an \ + optional dependency", feature, dep) } None => { - return Err(human(format!("Feature `{}` includes `{}` \ - which is neither a dependency \ - nor another feature", - feature, dep))) + bail!("Feature `{}` includes `{}` which is neither \ + a dependency nor another feature", feature, dep) } } } diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index 3688e958e..ef69e02c1 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -38,6 +38,12 @@ use term::color::{BLACK, RED}; pub use util::{CargoError, CliError, CliResult, human, Config, ChainError}; +macro_rules! bail { + ($($fmt:tt)*) => ( + return Err(::util::human(&format_args!($($fmt)*))) + ) +} + pub mod core; pub mod ops; pub mod sources; diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index cf588ae28..e89e842d8 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -32,7 +32,7 @@ pub fn clean(manifest_path: &Path, opts: &CleanOptions) -> CargoResult<()> { let source_id = root.package_id().source_id(); let resolve = match try!(ops::load_lockfile(&lockfile, source_id)) { Some(resolve) => resolve, - None => return Err(human("A Cargo.lock must exist before cleaning")) + None => bail!("a Cargo.lock must exist before cleaning") }; // Create a compilation context to have access to information like target diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 073d59293..f34de78f2 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -33,7 +33,7 @@ use core::{Profile, TargetKind, Profiles}; use core::resolver::{Method, Resolve}; use ops::{self, BuildOutput, ExecEngine}; use util::config::{ConfigValue, Config}; -use util::{CargoResult, internal, human, ChainError, profile}; +use util::{CargoResult, internal, ChainError, profile}; /// Contains information about how a package should be compiled. pub struct CompileOptions<'a> { @@ -156,7 +156,7 @@ pub fn compile_pkg<'a>(root_package: &Package, }).map(|s| s.to_string()).collect::>(); if jobs == Some(0) { - return Err(human("jobs must be at least 1")) + bail!("jobs must be at least 1") } let (packages, resolve_with_overrides, sources) = { @@ -177,8 +177,8 @@ pub fn compile_pkg<'a>(root_package: &Package, }; if spec.len() > 0 && invalid_spec.len() > 0 { - return Err(human(format!("could not find package matching spec `{}`", - invalid_spec.connect(", ")))); + bail!("could not find package matching spec `{}`", + invalid_spec.connect(", ")) } let to_builds = packages.iter().filter(|p| pkgids.contains(&p.package_id())) @@ -202,11 +202,9 @@ pub fn compile_pkg<'a>(root_package: &Package, profile.rustc_args = Some(args.to_vec()); general_targets.push((target, profile)); } else { - return Err(human("extra arguments to `rustc` can only be \ - passed to one target, consider \ - filtering\nthe package by passing e.g. \ - `--lib` or `--bin NAME` to specify \ - a single target")) + bail!("extra arguments to `rustc` can only be passed to one \ + target, consider filtering\nthe package by passing \ + e.g. `--lib` or `--bin NAME` to specify a single target") } } (None, Some(args)) => { @@ -218,11 +216,9 @@ pub fn compile_pkg<'a>(root_package: &Package, profile.rustdoc_args = Some(args.to_vec()); general_targets.push((target, profile)); } else { - return Err(human("extra arguments to `rustdoc` can only be \ - passed to one target, consider \ - filtering\nthe package by passing e.g. \ - `--lib` or `--bin NAME` to specify \ - a single target")) + bail!("extra arguments to `rustdoc` can only be passed to one \ + target, consider filtering\nthe package by passing e.g. \ + `--lib` or `--bin NAME` to specify a single target") } } (None, None) => { @@ -357,7 +353,7 @@ fn generate_targets<'a>(pkg: &'a Package, if let Some(t) = pkg.targets().iter().find(|t| t.is_lib()) { targets.push((t, profile)); } else { - return Err(human(format!("no library targets found"))) + bail!("no library targets found") } } @@ -369,9 +365,7 @@ fn generate_targets<'a>(pkg: &'a Package, }); let t = match target { Some(t) => t, - None => return Err(human(format!("no {} target \ - named `{}`", - desc, name))), + None => bail!("no {} target named `{}`", desc, name), }; debug!("found {} `{}`", desc, name); targets.push((t, profile)); @@ -429,11 +423,9 @@ fn scrape_build_config(config: &Config, let cfg_jobs = match try!(config.get_i64("build.jobs")) { Some((n, p)) => { if n <= 0 { - return Err(human(format!("build.jobs must be positive, \ - but found {} in {:?}", n, p))); + bail!("build.jobs must be positive, but found {} in {:?}", n, p) } else if n >= u32::max_value() as i64 { - return Err(human(format!("build.jobs is too large: \ - found {} in {:?}", n, p))); + bail!("build.jobs is too large: found {} in {:?}", n, p) } else { Some(n as u32) } diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index 296b792c0..8d843f31c 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -5,7 +5,7 @@ use std::process::Command; use core::{Package, PackageIdSpec}; use ops; -use util::{CargoResult, human}; +use util::CargoResult; pub struct DocOptions<'a> { pub open_result: bool, @@ -28,10 +28,9 @@ pub fn doc(manifest_path: &Path, } for bin in bin_names.iter() { if lib_names.contains(bin) { - return Err(human("Cannot document a package where a library \ - and a binary have the same name. Consider \ - renaming one or marking the target as \ - `doc = false`")) + bail!("cannot document a package where a library and a binary \ + have the same name. Consider renaming one or marking \ + the target as `doc = false`") } } } @@ -40,8 +39,7 @@ pub fn doc(manifest_path: &Path, if options.open_result { let name = if options.compile_opts.spec.len() > 1 { - return Err(human("Passing multiple packages and `open` is not \ - supported")) + bail!("Passing multiple packages and `open` is not supported") } else if options.compile_opts.spec.len() == 1 { try!(PackageIdSpec::parse(&options.compile_opts.spec[0])) .name().replace("-", "_").to_string() diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index ac5578e0c..cd876d8d5 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -6,8 +6,8 @@ use core::registry::PackageRegistry; use core::{Resolve, SourceId, Package}; use core::resolver::Method; use ops; -use util::config::{Config}; -use util::{CargoResult, human}; +use util::config::Config; +use util::CargoResult; pub struct UpdateOptions<'a> { pub config: &'a Config, @@ -33,12 +33,11 @@ pub fn update_lockfile(manifest_path: &Path, let previous_resolve = match try!(ops::load_pkg_lockfile(&package)) { Some(resolve) => resolve, - None => return Err(human("A Cargo.lock must exist before it is updated")) + None => bail!("a Cargo.lock must exist before it is updated") }; if opts.aggressive && opts.precise.is_some() { - return Err(human("cannot specify both aggressive and precise \ - simultaneously")) + bail!("cannot specify both aggressive and precise simultaneously") } let mut registry = PackageRegistry::new(opts.config); diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 4d1f8370a..f65490519 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -147,8 +147,8 @@ fn select_pkg<'a, T>(mut source: T, None => { match try!(one(examples, |v| multi_err("examples", v))) { Some(p) => p, - None => return Err(human("no packages found with \ - binaries or examples")), + None => bail!("no packages found with binaries or \ + examples"), } } }; @@ -201,7 +201,7 @@ fn check_overwrites(dst: &Path, // get checked during cargo_compile, we only care about the "build // everything" case here if pkg.targets().iter().filter(|t| t.is_bin()).next().is_none() { - return Err(human("specified package has no binaries")) + bail!("specified package has no binaries") } for target in pkg.targets().iter().filter(|t| t.is_bin()) { @@ -283,9 +283,8 @@ pub fn uninstall(root: Option<&str>, for bin in installed.get() { let bin = dst.join(bin); if fs::metadata(&bin).is_err() { - return Err(human(format!("corrupt metadata, `{}` does not \ - exist when it should", - bin.display()))) + bail!("corrupt metadata, `{}` does not exist when it should", + bin.display()) } } @@ -299,8 +298,7 @@ pub fn uninstall(root: Option<&str>, for bin in bins.iter() { if !installed.get().contains(bin) { - return Err(human(format!("binary `{}` not installed as part \ - of `{}`", bin, result))) + bail!("binary `{}` not installed as part of `{}`", bin, result) } } diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs index ef306f11f..4487f3166 100644 --- a/src/cargo/ops/cargo_new.rs +++ b/src/cargo/ops/cargo_new.rs @@ -47,8 +47,7 @@ struct CargoNewConfig { pub fn new(opts: NewOptions, config: &Config) -> CargoResult<()> { let path = config.cwd().join(opts.path); if fs::metadata(&path).is_ok() { - return Err(human(format!("Destination `{}` already exists", - path.display()))) + bail!("destination `{}` already exists", path.display()) } let name = match opts.name { Some(name) => name, @@ -74,8 +73,7 @@ pub fn new(opts: NewOptions, config: &Config) -> CargoResult<()> { for c in name.chars() { if c.is_alphanumeric() { continue } if c == '_' || c == '-' { continue } - return Err(human(&format!("Invalid character `{}` in crate name: `{}`", - c, name))); + bail!("Invalid character `{}` in crate name: `{}`", c, name) } mk(config, &path, name, &opts).chain_error(|| { human(format!("Failed to create project `{}` at `{}`", @@ -181,8 +179,8 @@ fn discover_author() -> CargoResult<(String, Option)> { Some(name) => name, None => { let username_var = if cfg!(windows) {"USERNAME"} else {"USER"}; - return Err(human(format!("could not determine the current \ - user, please set ${}", username_var))) + bail!("could not determine the current user, please set ${}", + username_var) } }; let email = git_config.and_then(|g| g.get_string("user.email").ok()) diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 76eab5396..b5539c1e8 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -136,8 +136,7 @@ fn tar(pkg: &Package, src: &PathSource, config: &Config, dst: &Path) -> CargoResult<()> { if fs::metadata(&dst).is_ok() { - return Err(human(format!("destination already exists: {}", - dst.display()))) + bail!("destination already exists: {}", dst.display()) } try!(fs::create_dir_all(dst.parent().unwrap())); @@ -241,15 +240,14 @@ fn check_filename(file: &Path) -> CargoResult<()> { let name = match name.to_str() { Some(name) => name, None => { - return Err(human(format!("path does not have a unicode filename \ - which may not unpack on all platforms: {}", - file.display()))) + bail!("path does not have a unicode filename which may not unpack \ + on all platforms: {}", file.display()) } }; let bad_chars = ['/', '\\', '<', '>', ':', '"', '|', '?', '*']; for c in bad_chars.iter().filter(|c| name.contains(**c)) { - return Err(human(format!("cannot package a filename with a special \ - character `{}`: {}", c, file.display()))) + bail!("cannot package a filename with a special character `{}`: {}", + c, file.display()) } Ok(()) } diff --git a/src/cargo/ops/cargo_pkgid.rs b/src/cargo/ops/cargo_pkgid.rs index 48fad309e..1d31a3015 100644 --- a/src/cargo/ops/cargo_pkgid.rs +++ b/src/cargo/ops/cargo_pkgid.rs @@ -2,7 +2,7 @@ use std::path::Path; use ops; use core::{PackageIdSpec, Package}; -use util::{CargoResult, human, Config}; +use util::{CargoResult, Config}; pub fn pkgid(manifest_path: &Path, spec: Option<&str>, @@ -13,7 +13,7 @@ pub fn pkgid(manifest_path: &Path, let source_id = package.package_id().source_id(); let resolve = match try!(ops::load_lockfile(&lockfile, source_id)) { Some(resolve) => resolve, - None => return Err(human("A Cargo.lock must exist for this command")) + None => bail!("a Cargo.lock must exist for this command"), }; let pkgid = match spec { diff --git a/src/cargo/ops/cargo_run.rs b/src/cargo/ops/cargo_run.rs index da576189a..05646e7e2 100644 --- a/src/cargo/ops/cargo_run.rs +++ b/src/cargo/ops/cargo_run.rs @@ -1,7 +1,7 @@ use std::path::Path; use ops::{self, ExecEngine, CompileFilter}; -use util::{self, CargoResult, human, process, ProcessError}; +use util::{self, CargoResult, process, ProcessError}; use core::Package; pub fn run(manifest_path: &Path, @@ -19,8 +19,7 @@ pub fn run(manifest_path: &Path, if bins.next().is_none() { match options.filter { CompileFilter::Everything => { - return Err(human("a bin target must be available for \ - `cargo run`")) + bail!("a bin target must be available for `cargo run`") } CompileFilter::Only { .. } => { // this will be verified in cargo_compile @@ -30,13 +29,13 @@ pub fn run(manifest_path: &Path, if bins.next().is_some() { match options.filter { CompileFilter::Everything => { - return Err(human("`cargo run` requires that a project only have \ - one executable; use the `--bin` option to \ - specify which one to run")) + bail!("`cargo run` requires that a project only have one \ + executable; use the `--bin` option to specify which one \ + to run") } CompileFilter::Only { .. } => { - return Err(human("`cargo run` can run at most one executable, \ - but multiple were specified")) + bail!("`cargo run` can run at most one executable, but \ + multiple were specified") } } } diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 086d2782a..d1aad6b5e 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -9,7 +9,6 @@ use core::{SourceMap, Package, PackageId, PackageSet, Resolve, Target, Profile}; use core::{TargetKind, LibKind, Profiles, Metadata, Dependency}; use core::dependency::Kind as DepKind; use util::{self, CargoResult, ChainError, internal, Config, profile}; -use util::human; use super::TargetConfig; use super::custom_build::{BuildState, BuildScripts}; @@ -196,8 +195,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { (&self.target_triple, &self.target_dylib) }; match *pair { - None => return Err(human(format!("dylib outputs are not supported \ - for {}", triple))), + None => bail!("dylib outputs are not supported for {}", triple), Some((ref s1, ref s2)) => Ok((s1, s2)), } } diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 8431e81c5..1fee049e2 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -269,8 +269,7 @@ impl BuildOutput { let (key, value) = match (key, value) { (Some(a), Some(b)) => (a, b.trim_right()), // line started with `cargo:` but didn't match `key=value` - _ => return Err(human(format!("Wrong output in {}: `{}`", - whence, line))) + _ => bail!("Wrong output in {}: `{}`", whence, line), }; match key { @@ -308,22 +307,20 @@ impl BuildOutput { None => break }; if flag != "-l" && flag != "-L" { - return Err(human(format!("Only `-l` and `-L` flags are allowed \ - in {}: `{}`", - whence, value))) + bail!("Only `-l` and `-L` flags are allowed in {}: `{}`", + whence, value) } let value = match flags_iter.next() { Some(v) => v, - None => return Err(human(format!("Flag in rustc-flags has no \ - value in {}: `{}`", - whence, value))) + None => bail!("Flag in rustc-flags has no value in {}: `{}`", + whence, value) }; match flag { "-l" => library_links.push(value.to_string()), "-L" => library_paths.push(PathBuf::from(value)), // was already checked above - _ => return Err(human("only -l and -L flags are allowed")) + _ => bail!("only -l and -L flags are allowed") }; } Ok((library_paths, library_links)) diff --git a/src/cargo/ops/cargo_rustc/links.rs b/src/cargo/ops/cargo_rustc/links.rs index 2021ffd75..e2a7ad4a8 100644 --- a/src/cargo/ops/cargo_rustc/links.rs +++ b/src/cargo/ops/cargo_rustc/links.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use core::{PackageId, PackageSet}; -use util::{CargoResult, human}; +use util::CargoResult; // Validate that there are no duplicated native libraries among packages and // that all packages with `links` also have a build script. @@ -13,34 +13,23 @@ pub fn validate(deps: &PackageSet) -> CargoResult<()> { Some(lib) => lib, None => continue, }; - match map.get(&lib) { - Some(previous) => { - let depid = dep.package_id(); - if previous.name() == depid.name() - && previous.source_id() == depid.source_id() { - return Err(human(format!("native library `{}` is being \ - linked to by more than one \ - version of the same package, but \ - it can only be linked \ - once; try updating \ - or pinning your dependencies to \ - ensure that this package only \ - shows up once\n\n {}\n {}", - lib, previous, dep.package_id()))) - } else { - return Err(human(format!("native library `{}` is being \ - linked to by more than one \ - package, and can only be linked \ - to by one package\n\n {}\n {}", - lib, previous, dep.package_id()))) - } + if let Some(prev) = map.get(&lib) { + let dep = dep.package_id(); + if prev.name() == dep.name() && prev.source_id() == dep.source_id() { + bail!("native library `{}` is being linked to by more \ + than one version of the same package, but it can \ + only be linked once; try updating or pinning your \ + dependencies to ensure that this package only shows \ + up once\n\n {}\n {}", lib, prev, dep) + } else { + bail!("native library `{}` is being linked to by more than \ + one package, and can only be linked to by one \ + package\n\n {}\n {}", lib, prev, dep) } - None => {} } if !dep.manifest().targets().iter().any(|t| t.is_custom_build()) { - return Err(human(format!("package `{}` specifies that it links to \ - `{}` but does not have a custom build \ - script", dep.package_id(), lib))) + bail!("package `{}` specifies that it links to `{}` but does not \ + have a custom build script", dep.package_id(), lib) } map.insert(lib, dep.package_id()); } diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index f9b6bf52b..4865d8d89 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -54,17 +54,14 @@ fn verify_dependencies(pkg: &Package, registry_src: &SourceId) for dep in pkg.dependencies().iter() { if dep.source_id().is_path() { if dep.specified_req().is_none() { - return Err(human(format!("all path dependencies must have \ - a version specified when \ - publishing.\n\ - dependency `{}` does not specify \ - a version", dep.name()))) + bail!("all path dependencies must have a version specified \ + when publishing.\ndependency `{}` does not specify \ + a version", dep.name()) } } else if dep.source_id() != registry_src { - return Err(human(format!("all dependencies must come from the \ - same source.\ndependency `{}` comes \ - from {} instead", dep.name(), - dep.source_id()))) + bail!("all dependencies must come from the same source.\n\ + dependency `{}` comes from {} instead", + dep.name(), dep.source_id()) } } Ok(()) @@ -99,8 +96,7 @@ fn transmit(pkg: &Package, tarball: &Path, registry: &mut Registry) match *license_file { Some(ref file) => { if fs::metadata(&pkg.root().join(file)).is_err() { - return Err(human(format!("the license file `{}` does not exist", - file))) + bail!("the license file `{}` does not exist", file) } } None => {} @@ -145,7 +141,7 @@ pub fn registry(config: &Config, let api_host = { let mut src = RegistrySource::new(&sid, config); try!(src.update().chain_error(|| { - human(format!("Failed to update registry {}", index)) + human(format!("failed to update registry {}", index)) })); (try!(src.config())).api }; @@ -319,7 +315,7 @@ pub fn yank(config: &Config, }; let version = match version { Some(v) => v, - None => return Err(human("a version must be specified to yank")) + None => bail!("a version must be specified to yank") }; let (mut registry, _) = try!(registry(config, token, index)); diff --git a/src/cargo/sources/registry.rs b/src/cargo/sources/registry.rs index 49d9cf207..7a7822a67 100644 --- a/src/cargo/sources/registry.rs +++ b/src/cargo/sources/registry.rs @@ -314,7 +314,7 @@ impl<'cfg> RegistrySource<'cfg> { // TODO: don't download into memory (curl-rust doesn't expose it) let resp = try!(handle.get(url.to_string()).follow_redirects(true).exec()); if resp.get_code() != 200 && resp.get_code() != 0 { - return Err(internal(format!("Failed to get 200 response from {}\n{}", + return Err(internal(format!("failed to get 200 response from {}\n{}", url, resp))) } @@ -325,8 +325,7 @@ impl<'cfg> RegistrySource<'cfg> { state.finish() }; if actual.to_hex() != expected_hash { - return Err(human(format!("Failed to verify the checksum of `{}`", - pkg))) + bail!("failed to verify the checksum of `{}`", pkg) } try!(paths::write(&dst, resp.get_body())); @@ -390,7 +389,7 @@ impl<'cfg> RegistrySource<'cfg> { .map(|l| self.parse_registry_package(l)) .collect(); try!(ret.chain_error(|| { - internal(format!("Failed to parse registry's information \ + internal(format!("failed to parse registry's information \ for: {}", name)) })) } @@ -544,11 +543,11 @@ impl<'cfg> Source for RegistrySource<'cfg> { url.path_mut().unwrap().push(package.version().to_string()); url.path_mut().unwrap().push("download".to_string()); let path = try!(self.download_package(package, &url).chain_error(|| { - internal(format!("Failed to download package `{}` from {}", + internal(format!("failed to download package `{}` from {}", package, url)) })); let path = try!(self.unpack_package(package, path).chain_error(|| { - internal(format!("Failed to unpack package `{}`", package)) + internal(format!("failed to unpack package `{}`", package)) })); let mut src = PathSource::new(&path, &self.source_id, self.config); try!(src.update()); diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 4fe7c8ddd..474b174f1 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -140,10 +140,9 @@ impl Config { let idx = key.split('.').take(i) .fold(0, |n, s| n + s.len()) + i - 1; let key_so_far = &key[..idx]; - return Err(human(format!("expected table for configuration \ - key `{}`, but found {} in {}", - key_so_far, val.desc(), - path.display()))); + bail!("expected table for configuration key `{}`, \ + but found {} in {}", + key_so_far, val.desc(), path.display()) } } } @@ -343,8 +342,8 @@ impl ConfigValue { Ok((key, value)) }).collect::>()), path.to_path_buf())) } - v => return Err(human(format!("found TOML configuration value of \ - unknown type `{}`", v.type_str()))) + v => bail!("found TOML configuration value of unknown type `{}`", + v.type_str()), } } diff --git a/src/cargo/util/important_paths.rs b/src/cargo/util/important_paths.rs index bca7fc3c2..5ee44cf20 100644 --- a/src/cargo/util/important_paths.rs +++ b/src/cargo/util/important_paths.rs @@ -29,8 +29,8 @@ pub fn find_project_manifest(pwd: &Path, file: &str) -> CargoResult { } } - Err(human(format!("Could not find `{}` in `{}` or any parent directory", - file, pwd.display()))) + bail!("could not find `{}` in `{}` or any parent directory", + file, pwd.display()) } /// Find the root Cargo.toml @@ -40,10 +40,10 @@ pub fn find_root_manifest_for_wd(manifest_path: Option, cwd: &Path) Some(path) => { let absolute_path = cwd.join(&path); if !absolute_path.ends_with("Cargo.toml") { - return Err(human("the manifest-path must be a path to a Cargo.toml file")) + bail!("the manifest-path must be a path to a Cargo.toml file") } if !fs::metadata(&absolute_path).is_ok() { - return Err(human(format!("manifest path `{}` does not exist", path))) + bail!("manifest path `{}` does not exist", path) } Ok(absolute_path) }, diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index 85a7befc0..4fe58ee8e 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -128,9 +128,9 @@ pub fn to_manifest(contents: &[u8], None => {} } if !manifest.targets().iter().any(|t| !t.is_custom_build()) { - return Err(human(format!("no targets specified in the manifest\n either \ - src/lib.rs, src/main.rs, a [lib] section, or [[bin]] \ - section must be present"))) + bail!("no targets specified in the manifest\n \ + either src/lib.rs, src/main.rs, a [lib] section, or [[bin]] \ + section must be present") } return Ok((manifest, paths)); @@ -374,11 +374,11 @@ impl TomlManifest { let project = self.project.as_ref().or_else(|| self.package.as_ref()); let project = try!(project.chain_error(|| { - human("No `package` or `project` section found.") + human("no `package` or `project` section found.") })); if project.name.trim().is_empty() { - return Err(human("package name cannot be an empty string.")) + bail!("package name cannot be an empty string.") } let pkgid = try!(project.to_package_id(source_id)); @@ -430,8 +430,8 @@ impl TomlManifest { for bin in bins.iter() { if blacklist.iter().find(|&x| *x == bin.name()) != None { - return Err(human(&format!("the binary target name `{}` is \ - forbidden", bin.name()))); + bail!("the binary target name `{}` is forbidden", + bin.name()) } } diff --git a/tests/support/mod.rs b/tests/support/mod.rs index e3fbc619a..01004ab6e 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -551,10 +551,6 @@ pub fn path2url(p: PathBuf) -> Url { Url::from_file_path(&*p).ok().unwrap() } -pub fn cwd() -> PathBuf { - env::current_dir().unwrap() -} - pub static RUNNING: &'static str = " Running"; pub static COMPILING: &'static str = " Compiling"; pub static DOCUMENTING: &'static str = " Documenting"; diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index 89b50ea0e..afdfed8e8 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -48,7 +48,7 @@ test!(cargo_compile_with_invalid_manifest { failed to parse manifest at `[..]` Caused by: - No `package` or `project` section found. + no `package` or `project` section found. ")) }); @@ -211,7 +211,7 @@ test!(cargo_compile_without_manifest { assert_that(p.cargo_process("build"), execs().with_status(101) .with_stderr("\ -Could not find `Cargo.toml` in `[..]` or any parent directory +could not find `Cargo.toml` in `[..]` or any parent directory ")); }); diff --git a/tests/test_cargo_doc.rs b/tests/test_cargo_doc.rs index fd223b00c..4ce4996c0 100644 --- a/tests/test_cargo_doc.rs +++ b/tests/test_cargo_doc.rs @@ -206,7 +206,7 @@ test!(doc_lib_bin_same_name { assert_that(p.cargo_process("doc"), execs().with_status(101) .with_stderr("\ -Cannot document a package where a library and a binary have the same name. \ +cannot document a package where a library and a binary have the same name. \ Consider renaming one or marking the target as `doc = false` ")); }); diff --git a/tests/test_cargo_new.rs b/tests/test_cargo_new.rs index 0745ebfff..7e8f8c023 100644 --- a/tests/test_cargo_new.rs +++ b/tests/test_cargo_new.rs @@ -87,7 +87,7 @@ test!(existing { fs::create_dir(&dst).unwrap(); assert_that(cargo_process("new").arg("foo"), execs().with_status(101) - .with_stderr(format!("Destination `{}` already exists\n", + .with_stderr(format!("destination `{}` already exists\n", dst.display()))); }); diff --git a/tests/test_cargo_registry.rs b/tests/test_cargo_registry.rs index a98cb99d5..064a417a4 100644 --- a/tests/test_cargo_registry.rs +++ b/tests/test_cargo_registry.rs @@ -166,10 +166,10 @@ test!(bad_cksum { unable to get packages from source Caused by: - Failed to download package `bad-cksum v0.0.1 (registry file://[..])` from [..] + failed to download package `bad-cksum v0.0.1 (registry file://[..])` from [..] Caused by: - Failed to verify the checksum of `bad-cksum v0.0.1 (registry file://[..])` + failed to verify the checksum of `bad-cksum v0.0.1 (registry file://[..])` ")); }); diff --git a/tests/test_cargo_rustdoc.rs b/tests/test_cargo_rustdoc.rs index 83084fc4f..0951d831f 100644 --- a/tests/test_cargo_rustdoc.rs +++ b/tests/test_cargo_rustdoc.rs @@ -169,7 +169,7 @@ test!(rustdoc_same_name_err { .arg("--").arg("--no-defaults"), execs() .with_status(101) - .with_stderr("Cannot document a package where a library and a \ + .with_stderr("cannot document a package where a library and a \ binary have the same name. Consider renaming one \ or marking the target as `doc = false`")); }); -- 2.30.2